feat: phase 5 - payments UI, notifications, CSV export, responsive layout#22
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughPhase 5 adds debtor detail/payment-recording sheets, supplier payment history, a notification bell aggregating overdue cheques/low-stock/credit alerts, CSV export across all three Reports tabs, and a responsive dashboard KPI grid. Alongside these features, all relative imports are normalized to ChangesPhase 5 Feature Work
Lint CI Workflow and Fixes
Sequence Diagram(s)sequenceDiagram
participant User
participant NotificationBell
participant notificationsProvider
participant ChequesDao
participant InventoryDao
participant CustomersDao
participant _AlertsPanel
User->>NotificationBell: tap bell icon
NotificationBell->>notificationsProvider: watch (auto-refresh)
notificationsProvider->>ChequesDao: getOverdueCheques()
notificationsProvider->>ChequesDao: due within 7 days
notificationsProvider->>InventoryDao: getLowStockProducts()
notificationsProvider->>CustomersDao: credit-exceeded customers
notificationsProvider-->>NotificationBell: List<AppAlert>
NotificationBell->>_AlertsPanel: showModalBottomSheet
_AlertsPanel->>_AlertsPanel: render per type (icon + color)
User->>_AlertsPanel: tap refresh
_AlertsPanel->>notificationsProvider: invalidate
sequenceDiagram
participant User
participant _DebtorTile
participant _DebtorDetailSheet
participant customerPaymentHistoryProvider
participant _RecordPaymentSheet
participant customerActionsProvider
User->>_DebtorTile: tap
_DebtorTile->>_DebtorDetailSheet: showModalBottomSheet
_DebtorDetailSheet->>customerPaymentHistoryProvider: watch(customerId)
customerPaymentHistoryProvider-->>_DebtorDetailSheet: List<Payment>
User->>_DebtorDetailSheet: tap "Record Payment"
_DebtorDetailSheet->>_RecordPaymentSheet: Navigator.push
User->>_RecordPaymentSheet: fill form + submit
_RecordPaymentSheet->>customerActionsProvider: recordPayment(...)
customerActionsProvider-->>_RecordPaymentSheet: success
_RecordPaymentSheet->>_RecordPaymentSheet: Navigator.pop + snackbar
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Preview deployed
Updates automatically on every push. |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/features/suppliers/presentation/suppliers_screen.dart (1)
130-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential crash on empty supplier name.
Line 67 in
_SupplierTilecorrectly checkssupplier.name.isNotEmptybefore accessingsupplier.name[0], but this detail sheet does not. If a supplier with an empty name exists, this will throw aRangeError.🛡️ Proposed fix
child: Text( - supplier.name[0].toUpperCase(), + supplier.name.isNotEmpty ? supplier.name[0].toUpperCase() : '?', style: const TextStyle(color: Colors.white, fontSize: 22, fontWeight: FontWeight.w700), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/suppliers/presentation/suppliers_screen.dart` at line 130, The code at line 130 accesses supplier.name[0] without verifying the supplier name is not empty, which can cause a RangeError if the supplier has an empty name. Add a guard condition to check supplier.name.isNotEmpty before accessing supplier.name[0].toUpperCase(), similar to the pattern already implemented in the _SupplierTile widget at line 67. If the name is empty, provide a sensible fallback such as using a default character or returning an empty string for the display logic.lib/providers/settings_provider.dart (1)
267-324:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the JSON import atomic before tightening casts.
The new casts at Lines 272, 288, and 302 can throw after earlier rows were already inserted. Since the catch returns
Import failed, wrap the import writes in a Drift transaction so malformed payloads do not leave a partial database restore.🛡️ Proposed fix
final payload = jsonDecode(utf8.decode(bytes)) as Map<String, dynamic>; final db = _db; - // Products - for (final Map<String, dynamic> p in ((payload['products'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { - await db.into(db.products).insert( - ProductsCompanion.insert( - id: p['id'] as String, - name: p['name'] as String, - unitType: Value((p['unit_type'] as String?) ?? 'pcs'), - costPrice: Value((p['cost_price'] as num?)?.toDouble() ?? 0), - sellPrice: Value((p['sell_price'] as num?)?.toDouble() ?? 0), - barcode: Value(p['barcode'] as String?), - brand: Value(p['brand'] as String?), - ), - mode: InsertMode.insertOrIgnore, - ); - } - - // Customers - for (final Map<String, dynamic> c in ((payload['customers'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { - await db.into(db.customers).insert( - CustomersCompanion.insert( - id: c['id'] as String, - name: c['name'] as String, - phone: Value(c['phone'] as String?), - address: Value(c['address'] as String?), - balance: Value((c['balance'] as num?)?.toDouble() ?? 0), - ), - mode: InsertMode.insertOrIgnore, - ); - } - - // Stock - for (final Map<String, dynamic> s in ((payload['stock'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { - await db.into(db.stock).insert( - StockCompanion.insert( - productId: s['product_id'] as String, - qty: Value((s['qty'] as num?)?.toDouble() ?? 0), - ), - mode: InsertMode.insertOrIgnore, - ); - } - - await _ref.read(auditLogDaoProvider).log( - id: _uuid.v7(), - entityType: 'database', - entityId: 'import', - action: 'create', - userId: _actorId, - userName: _actorName, - newValue: {'format': 'json', 'source': result.files.first.name}, - ); + await db.transaction(() async { + // Products + for (final Map<String, dynamic> p in ((payload['products'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { + await db.into(db.products).insert( + ProductsCompanion.insert( + id: p['id'] as String, + name: p['name'] as String, + unitType: Value((p['unit_type'] as String?) ?? 'pcs'), + costPrice: Value((p['cost_price'] as num?)?.toDouble() ?? 0), + sellPrice: Value((p['sell_price'] as num?)?.toDouble() ?? 0), + barcode: Value(p['barcode'] as String?), + brand: Value(p['brand'] as String?), + ), + mode: InsertMode.insertOrIgnore, + ); + } + + // Customers + for (final Map<String, dynamic> c in ((payload['customers'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { + await db.into(db.customers).insert( + CustomersCompanion.insert( + id: c['id'] as String, + name: c['name'] as String, + phone: Value(c['phone'] as String?), + address: Value(c['address'] as String?), + balance: Value((c['balance'] as num?)?.toDouble() ?? 0), + ), + mode: InsertMode.insertOrIgnore, + ); + } + + // Stock + for (final Map<String, dynamic> s in ((payload['stock'] as List?)?.cast<Map<String, dynamic>>() ?? [])) { + await db.into(db.stock).insert( + StockCompanion.insert( + productId: s['product_id'] as String, + qty: Value((s['qty'] as num?)?.toDouble() ?? 0), + ), + mode: InsertMode.insertOrIgnore, + ); + } + + await _ref.read(auditLogDaoProvider).log( + id: _uuid.v7(), + entityType: 'database', + entityId: 'import', + action: 'create', + userId: _actorId, + userName: _actorName, + newValue: {'format': 'json', 'source': result.files.first.name}, + ); + }); return 'Import complete';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/providers/settings_provider.dart` around lines 267 - 324, The multiple insert operations for products, customers, and stock tables are not wrapped in a database transaction, which means if a cast fails partway through, some inserts may have already succeeded leaving the database in a corrupted state. Wrap all the insert operations (the three for loops iterating over products, customers, and stock, as well as the audit log call) inside a Drift transaction to ensure atomicity. This way, if any error occurs during the import process, the entire transaction will be rolled back and the database will remain in a consistent state.lib/features/settings/presentation/settings_screen.dart (1)
476-485:⚠️ Potential issue | 🟠 MajorUse a non-null sentinel for the "All" audit filter option.
Selecting the "All" item (line 484) with no value specified results in
nullbeing returned. In Flutter'sPopupMenuButton,nullis treated as menu cancellation, soonSelectedis never invoked. This leaves_filterTypestuck on the previous selection instead of clearing the filter. Use a non-null sentinel value like'__all__'and map it back tonullin theonSelectedhandler.Proposed fix
- PopupMenuButton<String?>( + PopupMenuButton<String>( icon: Badge( isLabelVisible: _filterType != null, child: const Icon(Icons.filter_list), ), tooltip: 'Filter by type', - onSelected: (v) => setState(() => _filterType = v), + onSelected: (v) => setState(() => _filterType = v == '__all__' ? null : v), itemBuilder: (_) => [ - const PopupMenuItem(child: Text('All')), + const PopupMenuItem(value: '__all__', child: Text('All')), ..._types.map((t) => PopupMenuItem(value: t, child: Text(t))), ], ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/settings/presentation/settings_screen.dart` around lines 476 - 485, The "All" PopupMenuItem on line 484 has no value specified, which returns null and causes PopupMenuButton to treat it as a menu cancellation rather than a valid selection, preventing onSelected from being invoked and leaving _filterType unchanged. Assign a non-null sentinel value like '__all__' to the "All" PopupMenuItem, then modify the onSelected callback in the PopupMenuButton to check if the selected value equals '__all__' and map it to null when setting _filterType, ensuring the callback executes and the filter is properly cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 9-28: The Phase 5 section header includes a future date
(2026-06-18) that makes the unreleased version appear as if it's already been
released, which will cause confusion and drift if the release date changes.
Remove the date from the Phase 5 heading so it reads as either an undated phase
heading or change it to use the Unreleased designation, keeping only the version
information without the specific date until the release is finalized.
In `@lib/features/debtors/presentation/debtors_screen.dart`:
- Around line 219-225: The avatar section's Text widget directly accesses
customer.name[0] without checking if the name is empty, which will throw a
RangeError for customers with empty names. Add a guard condition in the Text
child to check if customer.name is not empty before accessing the first
character, and provide a fallback character (such as '?') when the name is
empty, matching the defensive pattern already implemented in _DebtorTile at line
155. This ensures consistency across the debtors_screen.dart file and prevents
runtime crashes.
In `@lib/features/pos/presentation/receipt_pdf.dart`:
- Around line 1-7: The receipt_pdf.dart file uses Uint8List.fromList(...) on
lines 49 and 52 but is missing the required import for Uint8List. Add the import
statement `import 'dart:typed_data';` to the import section at the top of the
file alongside the other existing imports, as Uint8List is not available through
any of the current imports including package:flutter/services.dart.
In `@lib/features/reports/presentation/reports_screen.dart`:
- Around line 186-199: The onPressed callback that calls _shareCsv() does not
handle potential failures from the share operation, leaving users without
feedback if something goes wrong. Make the onPressed callback an async function,
await the _shareCsv() call, and wrap it in a try-catch block. In the catch
block, display an error message to the user via a SnackBar or dialog to inform
them that the export failed. Apply this same pattern to all three export button
handlers mentioned in the comment.
- Around line 375-376: Create a utility function to properly escape CSV fields
by wrapping values in double quotes and escaping any internal quotes by doubling
them, and also strip or escape leading special characters (equals, plus, minus,
at-sign) that could trigger formula injection. Apply this sanitization function
to the r.name field in the map operation used for CSV export at the location
containing rows.map, and also apply the same function to any other
user-controlled field being interpolated into the CSV string at the second
occurrence around line 598-600. Ensure all string interpolations that include
user-provided data are passed through this sanitization function before being
added to the CSV output.
- Around line 98-99: The end date calculation in the date range for the report
(line 99) subtracts only 1 second from the next-month midnight, which excludes
records in the final milliseconds of the last day. Replace the Duration(seconds:
1) subtraction with Duration(milliseconds: 1) to capture the full end-of-day
precision and ensure all valid sales records are included when filtering with
isBetweenValues for both the P&L report and exported CSV data.
In `@lib/features/settings/presentation/settings_screen.dart`:
- Around line 1-13: The unconditional `import 'dart:io';` statement at the top
of settings_screen.dart causes compilation failures for Flutter web builds
because Dart's static compiler analysis rejects dart:io imports for web targets,
even though the File usage is guarded by kIsWeb at runtime in _downloadBytes.
Replace the static `import 'dart:io';` with a conditional import using `import
'dart:io' if (dart.library.html) 'dart:html';` to handle both IO and web
platforms, or extract the file download logic that depends on dart:io into a
separate platform-specific helper file and conditionally import the appropriate
implementation based on the target platform.
---
Outside diff comments:
In `@lib/features/settings/presentation/settings_screen.dart`:
- Around line 476-485: The "All" PopupMenuItem on line 484 has no value
specified, which returns null and causes PopupMenuButton to treat it as a menu
cancellation rather than a valid selection, preventing onSelected from being
invoked and leaving _filterType unchanged. Assign a non-null sentinel value like
'__all__' to the "All" PopupMenuItem, then modify the onSelected callback in the
PopupMenuButton to check if the selected value equals '__all__' and map it to
null when setting _filterType, ensuring the callback executes and the filter is
properly cleared.
In `@lib/features/suppliers/presentation/suppliers_screen.dart`:
- Line 130: The code at line 130 accesses supplier.name[0] without verifying the
supplier name is not empty, which can cause a RangeError if the supplier has an
empty name. Add a guard condition to check supplier.name.isNotEmpty before
accessing supplier.name[0].toUpperCase(), similar to the pattern already
implemented in the _SupplierTile widget at line 67. If the name is empty,
provide a sensible fallback such as using a default character or returning an
empty string for the display logic.
In `@lib/providers/settings_provider.dart`:
- Around line 267-324: The multiple insert operations for products, customers,
and stock tables are not wrapped in a database transaction, which means if a
cast fails partway through, some inserts may have already succeeded leaving the
database in a corrupted state. Wrap all the insert operations (the three for
loops iterating over products, customers, and stock, as well as the audit log
call) inside a Drift transaction to ensure atomicity. This way, if any error
occurs during the import process, the entire transaction will be rolled back and
the database will remain in a consistent state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9839ac7b-d55e-4452-8410-d01e747163b4
📒 Files selected for processing (69)
.github/workflows/lint.ymlCHANGELOG.mdlib/app.dartlib/core/constants/app_constants.dartlib/core/router/app_router.dartlib/core/router/route_guard.dartlib/core/theme/app_text_styles.dartlib/core/theme/app_theme.dartlib/core/utils/currency_utils.dartlib/core/utils/date_utils.dartlib/core/utils/logger.dartlib/data/database/app_database.dartlib/data/database/daos/audit_log_dao.dartlib/data/database/daos/cheques_dao.dartlib/data/database/daos/customers_dao.dartlib/data/database/daos/inventory_dao.dartlib/data/database/daos/invoices_dao.dartlib/data/database/daos/petty_cash_dao.dartlib/data/database/daos/reports_dao.dartlib/data/database/daos/returns_dao.dartlib/data/database/daos/suppliers_dao.dartlib/data/database/daos/users_dao.dartlib/data/database/tables/products_table.dartlib/data/database/tables/returns_table.dartlib/data/repositories/auth_repository.dartlib/data/repositories/inventory_repository.dartlib/features/auth/domain/auth_state.dartlib/features/auth/presentation/login_screen.dartlib/features/cheques/presentation/cheque_screen.dartlib/features/customers/presentation/customers_screen.dartlib/features/dashboard/presentation/dashboard_screen.dartlib/features/debtors/presentation/debtors_screen.dartlib/features/grn/presentation/grn_screen.dartlib/features/inventory/presentation/inventory_screen.dartlib/features/invoices/presentation/invoice_detail_screen.dartlib/features/invoices/presentation/invoice_pdf.dartlib/features/invoices/presentation/invoices_screen.dartlib/features/petty_cash/presentation/petty_cash_screen.dartlib/features/pos/presentation/pos_screen.dartlib/features/pos/presentation/receipt_pdf.dartlib/features/quick_sales/presentation/quick_sales_screen.dartlib/features/reports/presentation/reports_screen.dartlib/features/settings/presentation/settings_screen.dartlib/features/suppliers/presentation/suppliers_screen.dartlib/features/users/presentation/users_screen.dartlib/main.dartlib/providers/auth_provider.dartlib/providers/cheques_provider.dartlib/providers/customers_provider.dartlib/providers/dashboard_provider.dartlib/providers/database_provider.dartlib/providers/grn_provider.dartlib/providers/inventory_provider.dartlib/providers/invoices_provider.dartlib/providers/notifications_provider.dartlib/providers/petty_cash_provider.dartlib/providers/pos_provider.dartlib/providers/quick_sale_provider.dartlib/providers/settings_provider.dartlib/providers/suppliers_provider.dartlib/providers/users_provider.dartlib/shared/widgets/app_scaffold.dartlib/shared/widgets/bms_error_widget.dartlib/shared/widgets/bms_filter_bar.dartlib/shared/widgets/confirmation_dialog.dartlib/shared/widgets/notification_bell.dartlib/shared/widgets/sidebar_nav.dartlib/shared/widgets/stat_card.darttool/preview_receipt.dart
💤 Files with no reviewable changes (1)
- lib/core/utils/logger.dart
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@iamvirul pls check the coderabbit reviews |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Summary
Phase 5 adds customer/supplier payment UIs, an in-app notification bell, CSV export across all Reports tabs, responsive dashboard grid, a lint CI workflow, and resolves all 446 lint warnings.
Type
Changes
lint.ymlCI workflow enforcing zero issues withflutter analyze --fatal-infos --fatal-warningsRadio.groupValue/onChanged(migrated toRadioGroup), dynamic calls, positional boolean parameters, type errorsDecoratedBoxreplaced withMaterialto fix ink-splash assertionScreenshots
Test plan
flutter analyzereports no issuesRelated issues
Summary by CodeRabbit
Release Notes
New Features
Improvements